samples: sync NotesApp with latest ACI API changes#606
samples: sync NotesApp with latest ACI API changes#606surajkumar-08 wants to merge 7 commits intomicrosoft:release/experimentalfrom
Conversation
- Update namespace from Microsoft.Windows.AI.Search.Experimental.AppContentIndex to Microsoft.Windows.Search.AppContentIndex - Rename Remove -> RemoveContentItem, RemoveAll -> RemoveAllContentItems - Rename Subregion -> RegionOfInterest on image query matches - Update WinAppSDK from 2.0.0-experimental3 to 2.0.0-preview1 - Add IDisposable to SearchViewModel for proper cleanup - Remove stale commented-out internal project references from csproj - Fix README: correct solution filename, update SDK version reference - Clean up Package.appxmanifest identity for public sample Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Rename SearchViewModel._currentSearchCancellation (was missing '_' prefix, inconsistent with _searchText convention) - Call SearchViewModel.Dispose() via SearchView.Unloaded to release CancellationTokenSource when the control is torn down - Dispose AppContentIndexer in MainWindow.Closed handler so IClosable resources are released on window close - Replace int.Parse with int.TryParse for image match ContentId to avoid FormatException on unexpected content IDs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@abhmondal_microsoft can you please provide your review. |
|
@microsoft-github-policy-service agree company="Microsoft" |
| return null; | ||
| } | ||
|
|
||
| string fragment = match.GetType().GetProperty("Fragment")?.GetValue(match) as string ?? string.Empty; |
There was a problem hiding this comment.
The GetType().GetProperty("Fragment") pattern for reading OCR results is the only reflection-based property access in the entire samples repo. If the API renames that property in a future preview, this silently returns null and displays empty text with no error.
Is this a temporary workaround for a property not yet projected in the preview SDK? If so, could we add a // TODO: Replace with direct property access when projected comment so developers copying this sample know it's not the recommended pattern?
|
|
||
| private void TextQuerySession_ResultChanged(AppIndexTextQuerySession sender, object args) | ||
| { | ||
| _ = RefreshQuerySessionResultsAsync(); |
There was a problem hiding this comment.
_ = RefreshQuerySessionResultsAsync() discards the Task, so if the query session throws (e.g., after disposal or on an unsupported system), the exception vanishes and search silently stops working. Since this sample teaches developers how to use the Content Search API, would it be worth wrapping this in a try/catch with a logged or displayed error? Even a Debug.WriteLine would make failures visible when developers are adapting this code.
| <!-- Azure Settings Card --> | ||
| <toolkit:SettingsCard x:Name="AzureSettingsCard" Header="AzureSettings" Visibility="Collapsed" HorizontalContentAlignment="Left" ContentAlignment="Left"> | ||
| <StackPanel Margin="-42,-24,0,12" Spacing="8"> | ||
| <Border x:Name="AzureSettingsCard" |
There was a problem hiding this comment.
The switch from SettingsCard to Border removes the keyboard navigation and Narrator landmarks that SettingsCard provides out of the box. Since developers often copy sample XAML patterns directly, could we preserve accessibility here — either by keeping SettingsCard or adding AutomationProperties to the replacement controls?
3 participants
Summary
Update NotesApp sample to WinAppSDK 2.0.0-preview1 ACI API surface
Namespace & API renames
Microsoft.Windows.AI.Search.Experimental.AppContentIndextoMicrosoft.Windows.Search.AppContentIndexRemove→RemoveContentItem,RemoveAll→RemoveAllContentItemsRegionOfInterest(onAppManagedImageQueryMatch) instead ofSubregion(which remains onAppManagedOcrTextQueryMatchfor OCR matches)New API usage
GetIndexCapabilitiesOfCurrentSystem()before creating an indexAppContentIndexListenerevents (IndexCapabilitiesChanged,IndexStatisticsChanged,ContentItemStatusChanged) for live index monitoringIndexStatisticsChangedeventsGetIndexStatistics()on startupResource management & robustness
AppContentIndexerinMainWindow.ClosedhandlerIDisposabletoSearchViewModel; callDispose()viaSearchView.Unloadedto releaseCancellationTokenSourceint.Parsewithint.TryParsefor image matchContentIdto avoidFormatExceptionthrow new Exceptionwiththrow new InvalidOperationExceptionCleanup
Package.appxmanifestidentity for public sampleChecklist
Note that /azp run currently isn't working for this repo.